Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keyboard fixes #11586

Merged
merged 1 commit into from
Jun 12, 2017
Merged

Keyboard fixes #11586

merged 1 commit into from
Jun 12, 2017

Conversation

manucorporat
Copy link
Contributor

@manucorporat manucorporat commented May 9, 2017

DO NOT MERGE

fixes #7047
fixes #9699
fixes #11566
fixes #11484
fixes #11389
fixes #11325
fixes #11291
fixes #11251 ?
fixes #10552
fixes #10393
fixes #10183
fixes #10187

@@ -259,6 +259,7 @@ import { dateValueRange, renderDateTime, renderTextFormat, convertDataToISO, con
'ion-button="item-cover" ' +
'[attr.aria-labelledby]="_labelId" ' +
'[attr.aria-disabled]="_disabled" ' +
'(click)="_click($event)" ' +
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

@@ -119,7 +87,6 @@ input.text-input:-webkit-autofill {
pointer-events: auto;
}


Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

@@ -346,6 +346,7 @@ export class ItemSliding {
}
if (openAmount === 0) {
this._tmr = platform.timeout(() => {
// change in the code
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

@@ -134,6 +134,8 @@ export class RadioButton extends Ion implements IonicTapInput, OnDestroy, OnInit
}
}

initFocus() { }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

@@ -206,10 +206,6 @@ export class Select extends BaseInput<any> implements OnDestroy {

@HostListener('click', ['$event'])
_click(ev: UIEvent) {
if (ev.detail === 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review, ask @adamdbradley

/**
* @hidden
*/
focusNext() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is focusNext() used? need to review it
how does it interact between input, toggle, checkbox (look at items/inputs)

src/util/form.ts Outdated
const inputs = this._inputs;
let index = this._inputs.indexOf(currentInput) + 1;
if (index > 0 && index < inputs.length) {
let nextInput = inputs[index];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use var

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add unit tests! make sure I didn't break anything

@@ -68,6 +68,20 @@ export class ScrollView {
this._eventsEnabled = true;
}

setScrolling(isScrolling: boolean, ev: ScrollEvent) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review with @adamdbradley (explain use case)
add support for JS-scrolling? maybe not needed

add new unit tests

'<textarea #textInput *ngIf="_isTextarea" class="text-input" ' +
'[(ngModel)]="value" ' +
'(blur)="onBlur()" ' +
'(focus)="onFocus()" ' +
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could use non-zoned focusin/focusout programatically!!

this._usePadding = config.getBoolean('scrollPadding', this._useAssist);

if (elementRef.nativeElement.tagName === 'ION-TEXTAREA') {
this._type = TEXTAREA;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sure type has the correct value.

add unit test

_click(ev: UIEvent) {
// do not continue if the click event came from a form submit
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review, ask @adamdbradley

this._isFocus = true;
console.debug('BaseInput: focused:', this);
this._form && this._form.setAsFocused(this);
this._inputFocusChanged(true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might not need _inputFocusChanged

@Khalid-Nowaf
Copy link

Khalid-Nowaf commented May 26, 2017

Hi @manucorporat, do you think this PR can fix #11745 ?

@manucorporat manucorporat force-pushed the keyboard-fixes branch 6 times, most recently from f844f6c to 078dc62 Compare June 8, 2017 17:35
@manucorporat manucorporat force-pushed the keyboard-fixes branch 7 times, most recently from 7928f65 to 574ee31 Compare June 12, 2017 20:16
@manucorporat manucorporat merged commit c10f72b into ionic-team:master Jun 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment